Skip to content

Add (partial) safe protocol implementation for EFI_HII_DATABASE_PROTOCOL #1719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 5, 2025

Conversation

seijikun
Copy link
Contributor

@seijikun seijikun commented Jul 8, 2025

This only grants access to the HII-database's raw buffer for now.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@seijikun seijikun force-pushed the mr-hii-database branch 3 times, most recently from 144e2a9 to 917c7a0 Compare July 8, 2025 22:44
@seijikun
Copy link
Contributor Author

seijikun commented Jul 8, 2025

This is one of the last things we (IP-Projects) need ... for now.
Our end goal with this protocol is to be able to read the collection of available BIOS settings (+ their values), which requires to fully parse the hiidb datastructure.

Though I'm somewhat at a loss as to how to continue with this.
I think parsing hiidb without the help of some library is a big no-no, since this is what it looks like with a helping library.

Would it be acceptable to add a new dependency on something like binrw to add this to uefi-rs, or do you regard hiidb parsing as out-of-scope for uefi-rs alltogether?

@phip1611
Copy link
Member

phip1611 commented Jul 9, 2025

Would it be acceptable to add a new dependency on something like binrw to add this to uefi-rs, or do you regard hiidb parsing as out-of-scope for uefi-rs alltogether?

Not sure, we haven't done this so far. I have a light tendency towards no. In the documentation of the corresponding "high level" abstraction in uefi (pub struct HiiDatabase(HiiDatabaseProtocol);, you could add some more context for higher-level usages?

@seijikun seijikun force-pushed the mr-hii-database branch 3 times, most recently from e721b59 to eb3bfe1 Compare July 9, 2025 10:05
@seijikun seijikun force-pushed the mr-hii-database branch 2 times, most recently from c62f6ac to 2d4c7c8 Compare July 9, 2025 10:56
@nicholasbishop
Copy link
Member

I think parsing hiidb without the help of some library is a big no-no, since this is what it looks like with a helping library.

The HII data isn't something I'm very familiar with, so I don't have a strong opinion yet on adding a dep like binrw. I'd suggest as a first step, to try writing some representative code so that we can see what it looks like and put up an example PR. Feel free to use binrw or whatever other deps. Then we can evaluate how best to approach it. For example, we could decide to add it as an optional dep, so that it's only used when a hii feature is enabled, or even put hii parsing in an entirely separate package from uefi.

Copy link
Member

@nicholasbishop nicholasbishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. I rebased to fix the changelog conflict.

This only grants access to the HII-database's raw buffer for now.
@nicholasbishop nicholasbishop added this pull request to the merge queue Aug 5, 2025
Merged via the queue into rust-osdev:main with commit 2da288d Aug 5, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants